Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge my personal messages #62

Closed
wants to merge 9 commits into from
Closed

Conversation

violine1101
Copy link
Member

Since I plan on no longer using my personal database of messages, I added all messages that were not present in this repo yet and that I thought might be useful here.

Also includes some more improvements on existing messages. Please report back if you have any issues with what I changed.

Additionally, I have moved over some more commonly used things into variables, and compacted the existing variables down a bit. This also means that ⚠ this is a breaking change ⚠ since variables can include other variables now, which is also why I needed to adjust the JavaScript file a bit (recursion limit: 10). Other projects that rely on this may not expect that and might not be able to parse variables containing other variables correctly.

This PR is based on #61, so that should be merged before this one. I made a separate PR because this is a much larger change and is more likely to take longer to discuss.

@SPGoding
Copy link
Member

SPGoding commented May 23, 2020

As this is already a breaking change, how do you think about moving the new shortcuts to the keys? I don't feel like we need two different strings that can both be used to locate a certain message

}
// TODO: Allow more than one variable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just a small note: the comment here is actually for the message.fillname part)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, whoops, i thought it was about what I implemented.
Anyway, if that's really needed, it should probably be an issue at this point, but I don't really see the need for multiple inputs as of right now.

@violine1101
Copy link
Member Author

That's true. I wasn't sure if the keys were used for anything significant anywhere, so I kept them as it was.

Arisa is prepared now for this change, so it shouldn't be as breaking as it was anymore, it was a easier fix than I thought. Unless I've forgotten something else that relies on the helper messages, we should be good now.

If we go for breaking changes, it might make sense to revamp the format of the file completely. For instance on many messages you might just want to specify "all" projects. That's currently not possible. That could be fixed by for instance leaving out the "projects" property if all projects should be specified. And fillname must always be specified too, that's pretty annoying. But those things should probably be discussed in another issue.

@violine1101 violine1101 linked an issue May 24, 2020 that may be closed by this pull request
Copy link
Contributor

@Marcono1234 Marcono1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added some comments about sections which could possibly be improved.
Some have not been changed by this pull request, but if there will be such massive changes anyways, it might make sense to perform these smaller edits as well.

assets/messages.yml Outdated Show resolved Hide resolved
shortcut: crash
message: |-
*Thank you for your report!*
However, this issue is {color:#FF5722}*Invalid*{color}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it on purpose that this says the issue is invalid, while the message for bds, mcpe and realms does not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is. But I don't really want to tamper too much with the MCPE messages, I don't really know how they're used.

assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Outdated Show resolved Hide resolved
assets/messages.yml Show resolved Hide resolved
- mc
- mcl
name: Attach launcher log
shortcut: llog
Copy link
Member

@NeunEinser NeunEinser Jul 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply log? There doesn't seem to be any message that currently uses log as shortcut.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though log could be used for the game log in the future

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The game log is included in "Attach crash report".

Comment on lines +229 to 246
awaiting-response:
- project:
- bds
- mc
- mcl
- mcd
- mce
- mcpe
- mctest
- realms
- web
name: Awaiting Response
shortcut: ar
message: |-
%awaiting_response%

%quick_links%
fillname: []
Copy link
Member

@NeunEinser NeunEinser Jul 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure how this is intended to be used, but maybe it would make sense to add a variable to this one for the question being asked?


%quick_links%
fillname: []
duplicate-of-mcl-5638:
- project: mcl
name: Duplicate of MCL-5638
shortcut: jre
name: MCL-5638 / Unable to locate Java runtime
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did like those being sorted in the box together (because all duplicate messages used to start with "Duplicate".

But tbh, the dropdown becomes more and more crowded anyways, making it harder to find stuff, and we should probably think about a better solution in general.

Comment on lines +427 to 451
duplicate-resolved:
- project:
- bds
- mc
- mcapi
- mcd
- mce
- mcl
- mcpe
- mctest
- realms
- web
name: Duplicate (resolved)
shortcut: rdup
message: |-
*Thank you for your report!*
We're tracking this issue as *%s%*, so this ticket is being resolved and linked as a *duplicate*.

That ticket has already been resolved. Please take a look at it to learn more.

%search_for_dupes%

%quick_links%
fillname:
- Enter parent issue ID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need that message? We already have individual messages for fixed, tech support, wai and won't fix.

Other resolutions are:

  • Incomplete, but we don't use Incomplete tickets as a parent (or otherwise reopen it)
  • Cannot Reproduce, but that either means it's fixed, or it should be reopened
  • AR - might make sense to add a message for that saying something like "... but the parent is currently missing vital information. Please do take a look and see if you can help out, in order to get this bug fixed as soon as possible"
  • Done - is not used anymore afaik.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory we are missing one for "Invalid", though this message might not be that useful for this either?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we have one for tech support. Account issues obv. don't get linked, and a duplicate feature request doesn't happen often, and tbh I don't bother searching for a parent in this case and just resolve as invalid.

Comment on lines -829 to 972
Minecraft has several different editions, and some features vary from edition to edition. Often this happens when one edition evolves faster than another, and tweaks or balances are made that don't propagate immediately to other editions.
Parity issues are not tracked on the bug tracker but should instead be reported on the [Feedback website|https://feedback.minecraft.net/hc/en-us/community/topics/360001191251-Parity].
*Thank you for your report!*
However, this issue is {color:#FF5722}*Invalid*{color}.

You have posted a parity issue. Minecraft has several different editions, and some features vary from edition to edition.
This is especially the case with features that were added a long time ago, because one edition evolved faster than another, and tweaks or balances were made that did not propagate immediately to other editions.
These differences between the different versions of the game are called "Parity issues".

We only accept bug reports about a parity issue if it fulfills the following criteria:
* The feature affected by the parity issue is present in both Bedrock Edition and Java Edition in the latest release or development version
* The feature behaves differently in one edition than in the other
* The parity issue was introduced in Buzzy Bees (Bedrock Edition 1.14 / Java Edition 1.15) or later and was not present before

All other parity issues are not tracked on the bug tracker but should instead be reported on the [Feedback website|https://feedback.minecraft.net/hc/en-us/community/topics/360001191251-Parity].

For more information about this, please take a look at [this post|https://www.reddit.com/r/Mojira/comments/g9rjfh/from_now_on_well_accept_bug_reports_about_certain/] on our subreddit.

%quick_links%
fillname: []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That message has been changed already, is this change still required / better than the one we already have?

assets/messages.yml Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Oct 1, 2020

CLA assistant check
All committers have signed the CLA.

name: Attach hserr_pid file
shortcut: hserr
message: |-
Is there a file with a name starting with {{hs_err_pid}} in your {{[https://minecrafthopper.net/help/guides/finding-minecraft-data-folder/|.minecraft]}} folder? If so, please attach it here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also tell the user to make their report private, see MC-207889

@violine1101
Copy link
Member Author

This has diverged too much from the main branch and I also believe that the current messages are okay. I might still pick one or two of these changes up later but that would be separate, smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Contact Support" choice for REALMS project
5 participants